Add support for pluggable Regex Engines #112
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
=======================================
Coverage 99.74% 99.74%
=======================================
Files 28 29 +1
Lines 3126 3156 +30
=======================================
+ Hits 3118 3148 +30
Misses 8 8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
By default the underlying JSONSchema validator uses the standard Go regular expression engine; this engine supports the RE2 syntax which may result in failures during validation of OAS specifcations that use non-RE2 compliant patterns. The change introduces a configuration construct for each validator that may be passed at time of creation to affect the way it operates. Initially this construct allows for the regular expression engine to be overriden by a user-supplied variant. Closes pb33f#111
JemDay
changed the title
- Removed .gitignore - Fixed import ordering - Adopted 'Options pattern' for configuration.
|
@ccoVeille - Thanks for taking the time to poke through this ... the Options pattern will provide a much more fluent style and hopefully can be expanded in the future. |
You are welcome. Do you mind if I quote you and this PR on Dev.to article I plan to write in the next days? Here is my profile https://dev.to/ccoveille |
Sure (he said guardedly!) would appreciate cleaning this up first. |
- Updated documentation. - Fully adopted 'Options pattern'. - Addressed linting issues. - Stricter Unit Test assertions.
|
@ccoVeille - Thanks again for the comments If I get time tomorrow I'll see if it's possible to lift the config out completely and make it truly common without introducing circular dependencies and unintended publicly visible constructs. |
- Established new 'config' package - Establish a (hopefully) extensible mechanism and pattern to support config options.
- Less duplicative code. - Established a (hopefully) extenible model for any future config needs.
- Updated options structure name.
schema_validation/validate_schema.go
Outdated
| func NewSchemaValidatorWithLogger(logger *slog.Logger, opts ...config.Option) SchemaValidator { | ||
|
|
||
| options := config.NewValidationOptions(opts...) | ||
|
|
||
| return &schemaValidator{options: options, logger: logger, lock: sync.Mutex{}} | ||
|
|
||
| } | ||
|
|
||
| // NewSchemaValidator will create a new SchemaValidator instance, ready to accept schemas and payloads to validate. | ||
| func NewSchemaValidator() SchemaValidator { | ||
| func NewSchemaValidator(opts ...config.Option) SchemaValidator { | ||
| logger := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{ | ||
| Level: slog.LevelError, | ||
| })) | ||
| return NewSchemaValidatorWithLogger(logger) | ||
| return NewSchemaValidatorWithLogger(logger, opts...) | ||
| } |
There was a problem hiding this comment.
NewSchemaValidatorWithLogger was there before this PR, OK
But for me, you could consider to deprecate it and add a WithLogger Option
It doesn't have to be done in this PR, of course.
But I think it would be a huge improvement
But usually, the way to do that is to have a WithDefaultLogger(), that create the logger then it calls WithLogger(logger)
The WithDefaultLogger() is added in the options []Options slice as first argument.
Another way is to check is to do the opposite and appended on the options slice, but the WithDefaultLogger check if the logger is already defined and do nothing if present
There was a problem hiding this comment.
That would make sense ...
Thanks again for taking the time to poke through this, I guess the appetite for adopting the fluent style more broadly would rest with the owners.
| v.(*validator).document = document | ||
| return v, nil | ||
| } | ||
|
|
||
| // NewValidatorFromV3Model will create a new Validator from an OpenAPI Model | ||
| func NewValidatorFromV3Model(m *v3.Document) Validator { | ||
| func NewValidatorFromV3Model(m *v3.Document, opts ...config.Option) Validator { |
There was a problem hiding this comment.
Same with FromV3Model, that would become a WithV3Model(m)
|
@daveshanley - Any reservations/thoughts on this ? |
I have not had a chance to read through it yet to be honest, I have a few PRs to review and I am stuck on another set of libopenapi problems at the moment. I will get here as soon as I can. |
|
build seems to be failing linting |
Hmm ... will check .. |
Some files are not formatted using gofumpt, you can find the command in the makefile |
|
Linting issues resolved .. |
I am coming to the end of my latest feature run, I will switch back to PRs soon. Thank you for your patience. |
There was a problem hiding this comment.
This looks great, thank you for your contribution and thank you @ccoVeille for all your input!
By default the underlying JSONSchema validator uses the standard Go regular expression engine; this engine supports the RE2 syntax which may result in failures during validation of OAS specifications that use non-RE2 compliant patterns.
The change introduces a configuration construct for each validator that may be passed at time of creation to affect the way it operates.
Initially this construct allows for the regular expression engine to be overriden by a user-supplied variant.
Closes #111